Skip to content

[HLSL][RootSignature] Plug-in serialization and add full sample testcase #144769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 24, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented Jun 18, 2025

This pr extends dumpRootElements to invoke the print methods of all RootElements now that they are all implemented.

Extends the RootSignatures-AST.hlsl testcase to have a root element of each type being parsed, constructed to the in-memory representation mode and then being dumped as part of the AST dump.

  • Update HLSLRootSignatureUtils.cpp to extend dumpRootElements
  • Extend AST/HLSL/RootSigantures-AST.hlsl testcase
  • Defines the helper operator<< for RootElement
  • Small correction to the output of numDescriptors to be unbounded in special case

Resolves #124595.

@llvmbot llvmbot added clang Clang issues not falling into any other category HLSL HLSL Language Support labels Jun 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes

This pr extends dumpRootElements to invoke the print methods of all RootElements now that they are all implemented.

Extends the RootSignatures-AST.hlsl testcase to have a root element of each type being parsed, constructed to the in-memory representation mode and then being dumped as part of the AST dump.

  • Update HLSLRootSignatureUtils.cpp to extend dumpRootElements
  • Extend AST/HLSL/RootSigantures-AST.hlsl testcase

Resolves #124595.


Full diff: https://github.com/llvm/llvm-project/pull/144769.diff

2 Files Affected:

  • (modified) clang/test/AST/HLSL/RootSignatures-AST.hlsl (+69-28)
  • (modified) llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp (+24-18)
diff --git a/clang/test/AST/HLSL/RootSignatures-AST.hlsl b/clang/test/AST/HLSL/RootSignatures-AST.hlsl
index c700174da764d..1356dcc2fc87b 100644
--- a/clang/test/AST/HLSL/RootSignatures-AST.hlsl
+++ b/clang/test/AST/HLSL/RootSignatures-AST.hlsl
@@ -5,29 +5,61 @@
 // the Attr AST Node is created succesfully. If an invalid root signature was
 // passed in then we would exit out of Sema before the Attr is created.
 
-#define SampleRS \
-  "DescriptorTable( " \
-  "  CBV(b1), " \
-  "  SRV(t1, numDescriptors = 8, " \
-  "          flags = DESCRIPTORS_VOLATILE), " \
-  "  UAV(u1, numDescriptors = 0, " \
-  "          flags = DESCRIPTORS_VOLATILE) " \
-  "), " \
-  "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))"
+#define SampleRS "RootFlags( ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT | " \
+                             "DENY_VERTEX_SHADER_ROOT_ACCESS), " \
+                 "CBV(b0, space = 1, flags = DATA_STATIC), " \
+                 "SRV(t0), " \
+                 "UAV(u0), " \
+                 "DescriptorTable( CBV(b1), " \
+                                   "SRV(t1, numDescriptors = 8, " \
+                                   "        flags = DESCRIPTORS_VOLATILE), " \
+                                   "UAV(u1, numDescriptors = unbounded, " \
+                                   "        flags = DESCRIPTORS_VOLATILE)), " \
+                 "DescriptorTable(Sampler(s0, space=1, numDescriptors = 4)), " \
+                 "RootConstants(num32BitConstants=3, b10), " \
+                 "StaticSampler(s1)," \
+                 "StaticSampler(s2, " \
+                                 "addressU = TEXTURE_ADDRESS_CLAMP, " \
+                                 "filter = FILTER_MIN_MAG_MIP_LINEAR )"
 
 // CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[SAMPLE_RS_DECL:__hlsl_rootsig_decl_\d*]]
 // CHECK-SAME: RootElements{
-// CHECK-SAME:   CBV(b1, numDescriptors = 1, space = 0,
-// CHECK-SAME:     offset = DescriptorTableOffsetAppend, flags = DataStaticWhileSetAtExecute),
-// CHECK-SAME:   SRV(t1, numDescriptors = 8, space = 0,
-// CHECK-SAME:     offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile),
-// CHECK-SAME:   UAV(u1, numDescriptors = 0, space = 0,
-// CHECK-SAME:     offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile),
-// CHECK-SAME:   DescriptorTable(numClauses = 3, visibility = All),
-// CHECK-SAME:   Sampler(s0, numDescriptors = 4, space = 1,
-// CHECK-SAME:     offset = DescriptorTableOffsetAppend, flags = None),
-// CHECK-SAME:   DescriptorTable(numClauses = 1, visibility = All)
-// CHECK-SAME: }
+// CHECK-SAME: RootFlags(AllowInputAssemblerInputLayout | DenyVertexShaderRootAccess),
+// CHECK-SAME: RootCBV(b0,
+// CHECK-SAME:   space = 1, visibility = All, flags = DataStatic
+// CHECK-SAME: ),
+// CHECK-SAME: RootSRV(t0,
+// CHECK-SAME:   space = 0, visibility = All, flags = DataStaticWhileSetAtExecute
+// CHECK-SAME: ),
+// CHECK-SAME: RootUAV(
+// CHECK-SAME:   u0, space = 0, visibility = All, flags = DataVolatile
+// CHECK-SAME: ),
+// CHECK-SAME: CBV(
+// CHECK-SAME:   b1, numDescriptors = 1, space = 0, offset = DescriptorTableOffsetAppend, flags = DataStaticWhileSetAtExecute
+// CHECK-SAME: ),
+// CHECK-SAME: SRV(
+// CHECK-SAME:   t1, numDescriptors = 8, space = 0, offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile
+// CHECK-SAME: ),
+// CHECK-SAME: UAV(
+// CHECK-SAME:   u1, numDescriptors = 4294967295, space = 0, offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile
+// CHECK-SAME: ),
+// CHECK-SAME: DescriptorTable(
+// CHECK-SAME:   numClauses = 3, visibility = All
+// CHECK-SAME: ),
+// CHECK-SAME: Sampler(
+// CHECK-SAME:   s0, numDescriptors = 4, space = 1, offset = DescriptorTableOffsetAppend, flags = None
+// CHECK-SAME: ),
+// CHECK-SAME: DescriptorTable(
+// CHECK-SAME:   numClauses = 1, visibility = All
+// CHECK-SAME: ),
+// CHECK-SAME: RootConstants(
+// CHECK-SAME:   num32BitConstants = 3, b10, space = 0, visibility = All
+// CHECK-SAME: ),
+// CHECK-SAME: StaticSampler(
+// CHECK-SAME:   s1, filter = Anisotropic, addressU = Wrap, addressV = Wrap, addressW = Wrap,
+// CHECK-SAME:   mipLODBias = 0.000000e+00, maxAnisotropy = 16, comparisonFunc = LessEqual,
+// CHECK-SAME:   borderColor = OpaqueWhite, minLOD = 0.000000e+00, maxLOD = 3.402823e+38, space = 0, visibility = All
+// CHECK-SAME: )}
 
 // CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]]
 [RootSignature(SampleRS)]
@@ -44,14 +76,23 @@ void same_rs_main() {}
 // link to the same root signature declaration
 
 #define SampleSameRS \
-  "DescriptorTable( " \
-  "  CBV(b1), " \
-  "  SRV(t1, numDescriptors = 8, " \
-  "          flags = DESCRIPTORS_VOLATILE), " \
-  "  UAV(u1, numDescriptors = 0, " \
-  "          flags = DESCRIPTORS_VOLATILE) " \
-  "), " \
-  "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))"
+   "RootFlags( ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT | " \
+               "DENY_VERTEX_SHADER_ROOT_ACCESS), " \
+   "CBV(b0, space = 1, flags = DATA_STATIC), " \
+   "SRV(t0), " \
+   "UAV(u0), " \
+   "DescriptorTable( CBV(b1), " \
+                     "SRV(t1, numDescriptors = 8, " \
+                     "        flags = DESCRIPTORS_VOLATILE), " \
+                     "UAV(u1, numDescriptors = unbounded, " \
+                     "        flags = DESCRIPTORS_VOLATILE)), " \
+   "DescriptorTable(Sampler(s0, space=1, numDescriptors = 4)), " \
+   "RootConstants(num32BitConstants=3, b10), " \
+   "StaticSampler(s1)," \
+   "StaticSampler(s2, " \
+                   "addressU = TEXTURE_ADDRESS_CLAMP, " \
+                   "filter = FILTER_MIN_MAG_MIP_LINEAR )"
+
 
 // CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]]
 [RootSignature(SampleSameRS)]
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
index a1ddb318055be..52801741f351f 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
@@ -324,36 +324,42 @@ raw_ostream &operator<<(raw_ostream &OS, const StaticSampler &Sampler) {
   return OS;
 }
 
+namespace {
+
+// We use the OverloadVisit with std::visit to ensure the compiler catches if a
+// new RootElement variant type is added but it's operator<< or metadata
+// generation isn't handled.
+template <class... Ts> struct OverloadedVisit : Ts... {
+  using Ts::operator()...;
+};
+template <class... Ts> OverloadedVisit(Ts...) -> OverloadedVisit<Ts...>;
+
+} // namespace
+
 void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) {
-  OS << "RootElements{";
+  const auto Visitor = OverloadedVisit{
+      [&OS](const RootFlags &Flags) { OS << Flags; },
+      [&OS](const RootConstants &Constants) { OS << Constants; },
+      [&OS](const RootDescriptor &Descriptor) { OS << Descriptor; },
+      [&OS](const DescriptorTableClause &Clause) { OS << Clause; },
+      [&OS](const DescriptorTable &Table) { OS << Table; },
+      [&OS](const StaticSampler &Sampler) { OS << Sampler; },
+  };
+
+  OS << " RootElements{";
   bool First = true;
   for (const RootElement &Element : Elements) {
     if (!First)
       OS << ",";
     OS << " ";
-    if (const auto &Clause = std::get_if<DescriptorTableClause>(&Element))
-      OS << *Clause;
-    if (const auto &Table = std::get_if<DescriptorTable>(&Element))
-      OS << *Table;
+    std::visit(Visitor, Element);
     First = false;
   }
   OS << "}";
 }
 
-namespace {
-
-// We use the OverloadBuild with std::visit to ensure the compiler catches if a
-// new RootElement variant type is added but it's metadata generation isn't
-// handled.
-template <class... Ts> struct OverloadedBuild : Ts... {
-  using Ts::operator()...;
-};
-template <class... Ts> OverloadedBuild(Ts...) -> OverloadedBuild<Ts...>;
-
-} // namespace
-
 MDNode *MetadataBuilder::BuildRootSignature() {
-  const auto Visitor = OverloadedBuild{
+  const auto Visitor = OverloadedVisit{
       [this](const RootFlags &Flags) -> MDNode * {
         return BuildRootFlags(Flags);
       },

Copy link

github-actions bot commented Jun 23, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@inbelic inbelic force-pushed the inbelic/rs-plug-serialization branch from b07da56 to 29a8edb Compare June 23, 2025 23:09
@inbelic inbelic merged commit 310a62f into llvm:main Jun 24, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Update RootSignatures-AST.hlsl to have full sample testcase once parser is complete
4 participants